Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix: past spikes were not cleared in NEURON VectorSpikeSource #610

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

lkoelman
Copy link

@lkoelman lkoelman commented Nov 8, 2018

This fixed a bug for me when writing out data during a simulation. Without this line, spikes were not cleared and in recorder._get_current_segment an error was raised when constructing a neo.SpikeTrain containing spike times <= recording start time.

@coveralls
Copy link

Coverage Status

Coverage decreased (-20.4%) to 42.479% when pulling 340fcd6 on lkoelman:bug-temp1 into d2b629e on NeuralEnsemble:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-20.4%) to 42.479% when pulling 340fcd6 on lkoelman:bug-temp1 into d2b629e on NeuralEnsemble:master.

@coveralls
Copy link

coveralls commented Nov 8, 2018

Coverage Status

Coverage decreased (-20.4%) to 42.475% when pulling 48d608b on lkoelman:bug-temp1 into d2b629e on NeuralEnsemble:master.

Upon calling clear_past_spikes() the spike vector was resized,
but readout of those spike times by the VecStim mechanism was
not re-initialized. This caused the indexing calculation in
VecStim to go wrong resulting in erroneous spike timings.
@lkoelman
Copy link
Author

lkoelman commented Dec 2, 2018

I fixed a more serious bug that is more insidious and took me a while to track down. Basically the spikes coming out of a VectorSpikeSource are not valid anymore after you write out data during a simulation. This is because the Vector containing the spikes was resized, but the VecStim mechanism did not adjust its indexing calculation. Caused me a lot of headaches and lost simulation data so hope this gets merged quickly!

@apdavison
Copy link
Member

Many thanks for this. Please could you add tests for the two issues you've identified here? (in the file test/system/test_neuron.py)

@apdavison apdavison added this to the 0.9.3 milestone Dec 3, 2018
@lkoelman
Copy link
Author

lkoelman commented Dec 3, 2018

Uh-oh, seems like it is not fixed yet. VecStim does not reset the index to the correct value when calling play() again. I need to modify the VecStim mechanism to reset the index properly.

@apdavison apdavison modified the milestones: 0.9.3, 0.9.4 Dec 4, 2018
@apdavison apdavison modified the milestones: 0.9.4, 0.9.5 Mar 22, 2019
@apdavison apdavison modified the milestones: 0.9.5, 0.9.6 Jan 8, 2020
@apdavison apdavison modified the milestones: 0.9.6, 0.10.0 Nov 9, 2021
@apdavison apdavison modified the milestones: 0.10.0, 0.10.1 Dec 1, 2021
@apdavison apdavison modified the milestones: 0.10.1, 0.10.2 Oct 14, 2022
@apdavison apdavison modified the milestones: 0.10.2, 0.11.0 Feb 5, 2023
@apdavison apdavison modified the milestones: 0.11.0, 0.13.0 Feb 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants